Replace catching StackOverflowError with fuel#25937
Conversation
| T#T#T#T#T#T#T#T#T#T#T#T#T#T#T#T# | ||
| T#T#T#T#T#T#T#T#T#T#T#T#T#T#T#T# | ||
| T#T#T#T#T#T#T#T#T#T#T#T#T#T#T#T# | ||
| T#T#T#T#T#T#T#T#T#T#T#T#T#T#T#T# |
SolalPirelli
force-pushed
the
solal/fuel
branch
2 times, most recently
from
958aabd to
1e2b230
Compare
SolalPirelli
changed the title
SolalPirelli
force-pushed
the
solal/fuel
branch
3 times, most recently
from
e3ff246 to
44aa990
Compare
This comment was marked as outdated.
This comment was marked as outdated.
|
Benchmarks completed. Overview. |
|
|
||
| /** Ensures recursive operations obey the fuel limit, and throws user-friendly errors when they do not. */ | ||
| inline final def handleRecursive[T](name: String, details: => String, weight: Int = 1)(inline block: T): T = | ||
| val op = RecursiveOperation(name, details, weight) |
There was a problem hiding this comment.
also isnt this in the past this metadata only needed for the "outermost" operation, so perhaps another optimisation (rather than capturing a whole trace even in happy path)
There was a problem hiding this comment.
or tune the level of tracing captured via settings?
There was a problem hiding this comment.
we need the entire trace if something goes wrong, so we can show a message with the most frequent operations etc
but yeah the problem is fundamentally since we can't catch SOs we have to store info even in the happy path; I'll look at ways to lower the impact of this
There was a problem hiding this comment.
given that you actually still use the stack to recurse, cant you only construct the metadata when you throw the exception and then catch the exception from each outer layer to add its trace in and rethrow?
There was a problem hiding this comment.
yeah, I'm a little allergic to using exceptions for control flow, and it might negate the perf gains of not building the metadata, but we'll see if this latest benchmark run isn't good enough
|
Benchmarks started. Workflow run. |
3 participants
Part of #25799
How much have you relied on LLM-based tools in this contribution?
Not at all
How was the solution tested?
Covered by existing tests (this is a refactoring)